Add Evidence Management Dashboard#5057
Draft
rudransh-shrivastava wants to merge 54 commits into
Draft
Conversation
…aim-dash-cherry-claim
rudransh-shrivastava
added
the
gsoc2026:rudransh-shrivastava
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Summary by CodeRabbit
WalkthroughAdds claim and evidence GraphQL payloads, reusable action and form components, dashboard routes, and unit/e2e coverage. ChangesCandidate claim dashboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/bod-candidate-transparency #5057 +/- ##
======================================================================
- Coverage 98.47% 98.47% -0.01%
======================================================================
Files 550 556 +6
Lines 17675 17988 +313
Branches 2479 2537 +58
======================================================================
+ Hits 17405 17713 +308
- Misses 166 167 +1
- Partials 104 108 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/tests/unit/apps/owasp/api/internal/queries/board_candidate_claim_test.py`:
- Around line 29-30: The board candidate claim tests currently only stub the
queryset chain and do not verify the optimization contract. In the
`board_candidate_claims` test cases, assert that `annotate()` is called with an
`evidence_exists` keyword argument (along with the existing queryset chaining)
so the tests fail if the annotation is removed and the code falls back to
per-claim `.exists()` queries. Use the existing `mock_qs` setup in
`test_board_candidate_claims` to check the `annotate` call directly.
In `@e2e/helpers/mockClaimAuth.ts`:
- Around line 17-18: `mockClaimAuth` is unconditionally parsing request bodies
as JSON, which crashes on multipart `UploadHttpLink` uploads before the
`operationName` check. Update the `page.route('**/graphql/', ...)` handler to
inspect `request.headers()` first, only call `request.postDataJSON()` when the
content type is JSON, and safely skip or specially handle multipart requests so
evidence upload tests do not fail.
In `@e2e/pages/BoardCandidateClaimCreate.spec.ts`:
- Around line 8-22: The mocked createBoardCandidateClaim payload in
BoardCandidateClaimCreate.spec should match the GraphQL result shape used by the
cache update path. Update the CreateBoardCandidateClaimResult mock so it uses
the same claim object contract as production and includes the list fields
expected by GetBoardCandidateClaimsDocument, such as order and hasEvidence,
instead of returning a simplified BoardCandidateClaimNode shape. Keep the
response aligned with the createBoardCandidateClaim and claim symbols so the E2E
test exercises the real cache-append behavior.
In `@e2e/pages/BoardCandidateClaimEdit.spec.ts`:
- Around line 13-24: The edit E2E mock is using the wrong GraphQL typename for
updateBoardCandidateClaim. Update the mock in BoardCandidateClaimEdit.spec.ts to
match the shared ClaimResult contract instead of UpdateBoardCandidateClaimResult
so it stays aligned with the generated mutation and Apollo cache behavior. Keep
the existing update payload shape, but change the __typename on the
updateBoardCandidateClaim result to the real mutation result type used by the
schema.
In `@frontend/__tests__/mockData/mockClaimData.ts`:
- Around line 72-79: Keep mockSingleClaim aligned with the updated claim fixture
shape by adding the missing hasEvidence field so it matches mockClaims and the
current claim model. Update the mock in mockClaimData to include the same fields
used by the claim detail flow, and keep the object structure consistent with the
claim-related test data to avoid fixture drift.
In `@frontend/__tests__/mockData/mockEvidenceData.ts`:
- Around line 1-57: The claim/evidence test fixtures are duplicated in
mockEvidenceData and mockClaimData with slightly different shapes, which will
drift over time. Refactor the shared fixtures by introducing a single base
claim/evidence object or small builder used by both mockEvidence, mockEvidences,
mockClaimForEvidence, and the related mockGet*Data exports, and only override
fields like id and __typename where needed so the two test data modules stay
consistent.
In `@frontend/package.json`:
- Line 22: The new apollo-upload-client dependency in package.json is compatible
with ESM, but the repo’s Node requirement is pinned to ^24.0.0, so make sure the
CI/CD workflow and local dev setup are explicitly using Node 24. Update the
relevant environment/version config used by the frontend build and test jobs,
and verify any Node version manager or setup scripts align with the engines.node
constraint to prevent build or runtime mismatches.
In `@frontend/src/app/board/`[year]/candidates/[login]/claims/create/page.tsx:
- Around line 48-56: Handle candidateQueryError before the access-denied check
in the create claim page so transient fetch failures do not look like
authorization failures. Update the create page component to mirror the edit
page’s error handling by adding an explicit render branch for
candidateQueryError before computing or using isCandidate, and only fall through
to AccessDeniedDisplay when the query succeeded but the user is not allowed.
In `@frontend/src/app/board/`[year]/candidates/[login]/claims/page.tsx:
- Around line 137-157: The Apollo cache update in the reorder mutation is
targeting the wrong query document, so the UI stays stale after a successful
reorder. In the update callback inside the claims page, change both
cache.readQuery and cache.writeQuery to use GetBoardCandidateAndClaimsDocument
instead of GetBoardCandidateClaimsDocument, keeping the same login/year
variables, so the cached result matches the query actually used by the page and
derived state refreshes correctly.
In `@frontend/src/components/ClaimForm.tsx`:
- Around line 87-92: The catch block in ClaimForm’s submit flow is swallowing
mixed GraphQL failures by only checking hasValidationErrors. Update the error
handling around extractGraphQLErrors() to also inspect unmappedErrors, and only
suppress the exception when every GraphQL error was mapped to a field. If any
unmappedErrors remain, still call setBackendErrors(validationErrors) for the
mapped fields but rethrow the original error so the page-level toast path can
handle the unmapped/server error.
In `@frontend/src/components/EvidenceActions.tsx`:
- Around line 66-82: The cache update in EvidenceActions is targeting the wrong
GraphQL document, so the destination claim page stays stale and refetches
unnecessarily. Update the Apollo update handler in EvidenceActions to write the
removed evidence into the cache entry used by the claim details page, namely
GetClaimAndEvidencesDocument, not GetBoardCandidateClaimEvidencesDocument. Make
sure the variables and response shape match the page query in page.tsx
(including the nested boardCandidateClaim structure) so the evidence list
updates correctly after the redirect.
In `@frontend/src/components/EvidenceForm.tsx`:
- Line 66: Rename the typoed handler in EvidenceForm from handleFilechange to
handleFileChange to match standard camelCase and keep naming consistent; update
the function declaration and any references in the component so the event
handler symbol remains aligned everywhere.
- Around line 179-187: The file picker filtering in the EvidenceForm component
is using extension values without the required leading dots, so the native
accept filter is ineffective. Update the accept value passed to FormFileInput in
EvidenceForm to transform EVIDENCE_ALLOWED_EXTENSIONS into dotted extensions
before joining them, while keeping the existing runtime validation and
handleFilechange behavior unchanged.
In `@frontend/src/components/forms/shared/formValidationUtils.ts`:
- Around line 39-44: The file extension check in formValidationUtils is treating
extension-less filenames as if the whole filename were the extension, which
produces a misleading message. Update the validation logic around the file.name
split/pop handling to explicitly detect when there is no dot in the name and
return a clearer no-extension error before comparing against allowedExtensions.
While touching this block, use a separate allowed string in the same validation
helper to avoid the nested template literal warning in the return message.
In `@frontend/src/types/claim.ts`:
- Around line 3-14: The Claim type is weakening the GraphQL contract by making
backend-provided fields optional; update the frontend type to stay aligned with
the generated operation type instead of hand-typing these fields. In Claim, make
createdAt, hasEvidence, order, and updatedAt required (or replace this alias
with the generated GraphQL type used by the claims dashboard and cache updates)
so Claim, ClaimStatusEnum, and related consumers remain in lockstep.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bce348ca-a335-464e-be8e-96702ea37bc7
⛔ Files ignored due to path filters (7)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlfrontend/src/types/__generated__/boardQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/claimMutations.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/claimQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/evidenceMutations.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/evidenceQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**
📒 Files selected for processing (54)
backend/apps/owasp/api/internal/mutations/board_candidate_claim.pybackend/apps/owasp/api/internal/nodes/board_candidate_claim.pybackend/apps/owasp/api/internal/queries/board_candidate_claim.pybackend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_test.pybackend/tests/unit/apps/owasp/api/internal/nodes/board_candidate_claim_test.pybackend/tests/unit/apps/owasp/api/internal/queries/board_candidate_claim_test.pye2e/helpers/mockAuthCookies.tse2e/helpers/mockClaimAuth.tse2e/helpers/mockDashboardCookies.tse2e/pages/BoardCandidateClaimCreate.spec.tse2e/pages/BoardCandidateClaimDetails.spec.tse2e/pages/BoardCandidateClaimEdit.spec.tse2e/pages/BoardCandidateClaimEvidenceCreate.spec.tse2e/pages/BoardCandidateClaimEvidenceDetails.spec.tse2e/pages/BoardCandidateClaimEvidenceEdit.spec.tse2e/pages/BoardCandidateClaims.spec.tsfrontend/__tests__/mockData/mockClaimData.tsfrontend/__tests__/mockData/mockEvidenceData.tsfrontend/__tests__/unit/components/ActionButton.test.tsxfrontend/__tests__/unit/components/ClaimActions.test.tsxfrontend/__tests__/unit/components/DropdownActions.test.tsxfrontend/__tests__/unit/components/EvidenceActions.test.tsxfrontend/__tests__/unit/components/EvidenceForm.test.tsxfrontend/__tests__/unit/pages/BoardCandidatesPage.test.tsxfrontend/__tests__/unit/pages/CandidateClaimsPage.test.tsxfrontend/__tests__/unit/pages/ClaimDetailsPage.test.tsxfrontend/__tests__/unit/pages/CreateClaimPage.test.tsxfrontend/__tests__/unit/pages/CreateEvidencePage.test.tsxfrontend/__tests__/unit/pages/EditClaimPage.test.tsxfrontend/__tests__/unit/pages/EditEvidencePage.test.tsxfrontend/__tests__/unit/pages/EvidenceDetailsPage.test.tsxfrontend/package.jsonfrontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/edit/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/evidences/[evidenceKey]/edit/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/evidences/[evidenceKey]/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/evidences/create/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/page.tsxfrontend/src/app/board/[year]/candidates/page.tsxfrontend/src/components/ClaimActions.tsxfrontend/src/components/ClaimForm.tsxfrontend/src/components/DropdownActions.tsxfrontend/src/components/EvidenceActions.tsxfrontend/src/components/EvidenceForm.tsxfrontend/src/components/forms/shared/FormFileInput.tsxfrontend/src/components/forms/shared/formValidationUtils.tsfrontend/src/server/mutations/claimMutations.tsfrontend/src/server/mutations/evidenceMutations.tsfrontend/src/server/queries/boardQueries.tsfrontend/src/server/queries/claimQueries.tsfrontend/src/server/queries/evidenceQueries.tsfrontend/src/types/claim.tsfrontend/src/utils/helpers/apolloClient.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
14 issues found across 61 files
Confidence score: 2/5
frontend/package.jsondowngradesdompurifyfrom 3.4.11 to 3.4.9, which reintroduces a known sanitizer vulnerability; merging as-is reopens a concrete security risk in HTML sanitization paths—restore 3.4.11+ before merging.frontend/src/components/forms/shared/FormFileInput.tsxhas a behavior/accessibility mismatch (requiredis visual-only, and errors are not connected viaaria-describedby/aria-invalid), so users can submit without a file and assistive tech may miss validation feedback—wire the required/error props through to the native input before merge.- File/URL validation flow is inconsistent across
frontend/src/components/forms/shared/formValidationUtils.tsandfrontend/src/components/EvidenceForm.tsx(dotless filenames treated as extensions, andsourceUrlbackend errors can persist after file selection), which can leave users stuck in confusing or stale error states—fix extension parsing and clear the URL error when file selection satisfies the requirement. - A few interaction paths can fail confusingly in production: empty-action menus in
frontend/src/components/DropdownActions.tsx,NaNyearvariables infrontend/src/components/EvidenceActions.tsx, and query failures shown as “Access Denied” infrontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsx—add guards and explicit error handling so failures are recoverable and correctly messaged before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/DropdownActions.tsx">
<violation number="1" location="frontend/src/components/DropdownActions.tsx:20">
P3: New component duplicates existing dropdown behavior already implemented in `EntityActions`. Keeping both copies increases drift risk for future accessibility/keyboard fixes.</violation>
</file>
<file name="frontend/src/components/ClaimForm.tsx">
<violation number="1" location="frontend/src/components/ClaimForm.tsx:41">
P3: This file duplicates shared form-state and submit-error logic already present in other forms. Extracting a shared hook/helper would reduce maintenance drift across forms.</violation>
</file>
<file name="frontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/evidences/[evidenceKey]/page.tsx">
<violation number="1" location="frontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/evidences/[evidenceKey]/page.tsx:102">
P2: Null file URL path is silent; download click can appear broken. Show an error toast when URL is missing.</violation>
</file>
<file name="frontend/src/server/mutations/claimMutations.ts">
<violation number="1" location="frontend/src/server/mutations/claimMutations.ts:10">
P2: Repeated field selections across all six mutations should use a shared GraphQL fragment to keep fields in sync and reduce maintenance burden.</violation>
</file>
<file name="frontend/src/components/EvidenceActions.tsx">
<violation number="1" location="frontend/src/components/EvidenceActions.tsx:63">
P2: Validate `year` before using it in mutation/query variables. Passing `NaN` into required `Int` variables can break evidence removal and cache updates.</violation>
<violation number="2" location="frontend/src/components/EvidenceActions.tsx:111">
P3: Use strict equality for status checks. Loose equality here is unnecessary and can cause coercion-based false positives.</violation>
</file>
<file name="frontend/src/components/EvidenceForm.tsx">
<violation number="1" location="frontend/src/components/EvidenceForm.tsx:76">
P2: `sourceUrl` backend error is not cleared when a file is selected. Users can still see an invalid error state after satisfying the "file or URL" requirement.</violation>
</file>
<file name="frontend/src/components/forms/shared/FormFileInput.tsx">
<violation number="1" location="frontend/src/components/forms/shared/FormFileInput.tsx:12">
P2: `required` prop shows a visual asterisk but is not passed to `<input type="file">`. Browser won't enforce file selection, creating a mismatch between visual indication and actual behavior.</violation>
<violation number="2" location="frontend/src/components/forms/shared/FormFileInput.tsx:42">
P2: Error message not linked to input via `aria-describedby` or `aria-invalid`. Screen readers won't announce the error when focused on the file input.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 40 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
| }) | ||
| return | ||
| } | ||
| if (url) { |
Contributor
There was a problem hiding this comment.
P2: Null file URL path is silent; download click can appear broken. Show an error toast when URL is missing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/evidences/[evidenceKey]/page.tsx, line 102:
<comment>Null file URL path is silent; download click can appear broken. Show an error toast when URL is missing.</comment>
<file context>
@@ -0,0 +1,134 @@
+ })
+ return
+ }
+ if (url) {
+ const a = document.createElement('a')
+ a.href = url
</file context>
| } else { | ||
| setFileError(undefined) | ||
| } | ||
| if (backendErrors['file']) { |
Contributor
There was a problem hiding this comment.
P2: sourceUrl backend error is not cleared when a file is selected. Users can still see an invalid error state after satisfying the "file or URL" requirement.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/EvidenceForm.tsx, line 76:
<comment>`sourceUrl` backend error is not cleared when a file is selected. Users can still see an invalid error state after satisfying the "file or URL" requirement.</comment>
<file context>
@@ -0,0 +1,195 @@
+ } else {
+ setFileError(undefined)
+ }
+ if (backendErrors['file']) {
+ setBackendErrors((prev) => {
+ const next = { ...prev }
</file context>
Contributor
There was a problem hiding this comment.
1 issue found across 22 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/evidences/[evidenceKey]/page.tsx">
<violation number="1" location="frontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/evidences/[evidenceKey]/page.tsx:102">
P2: Null file URL path is silent; download click can appear broken. Show an error toast when URL is missing.</violation>
</file>
<file name="frontend/src/components/EvidenceActions.tsx">
<violation number="1" location="frontend/src/components/EvidenceActions.tsx:63">
P2: Validate `year` before using it in mutation/query variables. Passing `NaN` into required `Int` variables can break evidence removal and cache updates.</violation>
</file>
<file name="frontend/src/components/EvidenceForm.tsx">
<violation number="1" location="frontend/src/components/EvidenceForm.tsx:76">
P2: `sourceUrl` backend error is not cleared when a file is selected. Users can still see an invalid error state after satisfying the "file or URL" requirement.</violation>
</file>
<file name="frontend/src/components/forms/shared/FormFileInput.tsx">
<violation number="1" location="frontend/src/components/forms/shared/FormFileInput.tsx:12">
P2: `required` prop shows a visual asterisk but is not passed to `<input type="file">`. Browser won't enforce file selection, creating a mismatch between visual indication and actual behavior.</violation>
<violation number="2" location="frontend/src/components/forms/shared/FormFileInput.tsx:42">
P2: Error message not linked to input via `aria-describedby` or `aria-invalid`. Screen readers won't announce the error when focused on the file input.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
frontend/src/components/EvidenceActions.tsx (1)
66-89: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDon't update the cache before the mutation is confirmed successful.
Line 86 checks
okonly after Apollo has already executed theupdatecallback from Line 66. A response like{ ok: false }will still remove the evidence fromGetClaimAndEvidencesDocument, so the toast says the removal failed while the cached list has already dropped the item.Proposed change
update(cache, { data }) { + if (!data?.removeBoardCandidateClaimEvidence?.ok) return + const existing = cache.readQuery<GetClaimAndEvidencesQuery>({ query: GetClaimAndEvidencesDocument, variables: { key: claim.key, login, year: Number.parseInt(year) }, })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/EvidenceActions.tsx` around lines 66 - 89, The cache update in EvidenceActions.remove evidence flow is happening too early, before the mutation result is verified. Move the Apollo cache write logic out of the mutation’s update callback and apply it only after confirming removeBoardCandidateClaimEvidence.ok is true, using the existing GetClaimAndEvidencesDocument cache read/write path so failed removals do not remove the item from the cached list.frontend/src/app/board/[year]/candidates/[login]/claims/page.tsx (1)
44-50: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a visible focusable button for the reorder control.
The interactive element here is a fully transparent input, so keyboard focus has no visible indicator. That makes claim reordering hard for sighted keyboard users.
Proposed change
- <div className="relative rounded p-2 hover:bg-gray-200 dark:hover:bg-gray-700"> - <input - type="button" + <button + type="button" aria-label={direction === 'up' ? 'Move claim up' : 'Move claim down'} - className="absolute inset-0 cursor-pointer opacity-0" + className="rounded p-2 hover:bg-gray-200 focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-blue-500 dark:hover:bg-gray-700" onClick={handleClick} - /> + > <Icon className="pointer-events-none text-gray-400 dark:text-gray-500" size={24} /> - </div> + </button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/board/`[year]/candidates/[login]/claims/page.tsx around lines 44 - 50, The reorder control in the claim row uses a fully transparent input, so it has no visible focus state for keyboard users. Update the reorder UI in the claims page component to use a visible, focusable button element for the action handled by handleClick, and ensure the button keeps the same aria-label and click behavior while adding an obvious focus indicator for sighted keyboard navigation.frontend/src/components/DropdownActions.tsx (1)
26-38: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winOnly attach the document listener while the menu can actually be open.
Line 46 returns
null, but the effect from Line 26 still subscribes on mount. Because this component is reused per claim/evidence row, closed or actionless rows all keep a document-levelmousedownhandler alive.Proposed change
useEffect(() => { + if (!dropdownOpen || options.length === 0) return + const handleClickOutside = (event: MouseEvent) => { if (dropdownRef.current && !dropdownRef.current.contains(event.target as Node)) { setDropdownOpen(false) setFocusIndex(-1) } } document.addEventListener('mousedown', handleClickOutside) return () => { document.removeEventListener('mousedown', handleClickOutside) } - }, []) + }, [dropdownOpen, options.length])Also applies to: 46-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/DropdownActions.tsx` around lines 26 - 38, The DropdownActions useEffect currently registers a document-level mousedown listener on mount even when the component returns null and cannot open, which leaves unnecessary handlers alive for closed/actionless rows. Update the listener setup in DropdownActions to only subscribe when the menu is actually available/openable (for example by guarding on the same condition that allows rendering or opening), and keep the cleanup in place so the handler is removed when that condition changes or the component unmounts.e2e/helpers/mockClaimAuth.ts (1)
44-49: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFail closed when
operationNamecannot be parsed.With an allowlist, unparseable GraphQL requests currently fall through to
json: { data: mockData }. That can make a renamed or malformed multipart operation look successful instead of failing the test.Suggested fix
- } else if (operationNames && operationName && !operationNames.includes(operationName)) { + } else if (operationNames && (!operationName || !operationNames.includes(operationName))) { await route.abort('aborted') } else { await route.fulfill({ status: 200, json: { data: mockData },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/helpers/mockClaimAuth.ts` around lines 44 - 49, Fail closed in mockClaimAuth when the GraphQL operation name cannot be parsed, instead of letting it reach the route.fulfill fallback. Update the branching around the operationName/operationNames allowlist check so unparseable requests are treated like disallowed operations and aborted before returning mockData, using the existing mockClaimAuth route handler logic.frontend/src/components/EvidenceForm.tsx (1)
153-185: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSurface backend validation errors for
descriptionandfile.
handleSubmitstores GraphQL field errors inbackendErrors, but this render path only reads that map fornameandsourceUrl. Server-sidedescriptionorfileerrors are therefore dropped, so the submit can fail without showing the user what to fix.Suggested fix
<FormTextarea id="evidence-description" label="Description" placeholder="Enter evidence description" value={formData.description} onChange={(e) => { handleInputChange('description', e.target.value) setTouched((prev) => ({ ...prev, description: true })) }} - error={errors.description} + error={errors.description ?? backendErrors.description} touched={touched.description} required /> @@ <FormFileInput id="evidence-file" label="File (optional)" onChange={handleFileChange} accept={EVIDENCE_ALLOWED_EXTENSIONS.map((e) => '.' + e).join(',')} selectedFile={formData.file} - error={fileError} + error={fileError ?? backendErrors.file} touched={true} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/EvidenceForm.tsx` around lines 153 - 185, The submit flow in EvidenceForm is only surfacing backendErrors for name and sourceUrl, so GraphQL validation failures for description and file are hidden. Update the render path in EvidenceForm to read backendErrors.description and backendErrors.file for the corresponding FormTextarea and FormFileInput, and ensure handleSubmit continues mapping server field errors into backendErrors so those messages appear after submission.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@e2e/helpers/mockClaimAuth.ts`:
- Around line 44-49: Fail closed in mockClaimAuth when the GraphQL operation
name cannot be parsed, instead of letting it reach the route.fulfill fallback.
Update the branching around the operationName/operationNames allowlist check so
unparseable requests are treated like disallowed operations and aborted before
returning mockData, using the existing mockClaimAuth route handler logic.
In `@frontend/src/app/board/`[year]/candidates/[login]/claims/page.tsx:
- Around line 44-50: The reorder control in the claim row uses a fully
transparent input, so it has no visible focus state for keyboard users. Update
the reorder UI in the claims page component to use a visible, focusable button
element for the action handled by handleClick, and ensure the button keeps the
same aria-label and click behavior while adding an obvious focus indicator for
sighted keyboard navigation.
In `@frontend/src/components/DropdownActions.tsx`:
- Around line 26-38: The DropdownActions useEffect currently registers a
document-level mousedown listener on mount even when the component returns null
and cannot open, which leaves unnecessary handlers alive for closed/actionless
rows. Update the listener setup in DropdownActions to only subscribe when the
menu is actually available/openable (for example by guarding on the same
condition that allows rendering or opening), and keep the cleanup in place so
the handler is removed when that condition changes or the component unmounts.
In `@frontend/src/components/EvidenceActions.tsx`:
- Around line 66-89: The cache update in EvidenceActions.remove evidence flow is
happening too early, before the mutation result is verified. Move the Apollo
cache write logic out of the mutation’s update callback and apply it only after
confirming removeBoardCandidateClaimEvidence.ok is true, using the existing
GetClaimAndEvidencesDocument cache read/write path so failed removals do not
remove the item from the cached list.
In `@frontend/src/components/EvidenceForm.tsx`:
- Around line 153-185: The submit flow in EvidenceForm is only surfacing
backendErrors for name and sourceUrl, so GraphQL validation failures for
description and file are hidden. Update the render path in EvidenceForm to read
backendErrors.description and backendErrors.file for the corresponding
FormTextarea and FormFileInput, and ensure handleSubmit continues mapping server
field errors into backendErrors so those messages appear after submission.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6bd44b3b-f163-4e5c-befa-71c83f5aa906
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
backend/apps/owasp/api/internal/queries/board_candidate_claim.pybackend/tests/unit/apps/owasp/api/internal/queries/board_candidate_claim_test.pye2e/helpers/mockClaimAuth.tse2e/pages/BoardCandidateClaimCreate.spec.tse2e/pages/BoardCandidateClaimEdit.spec.tsfrontend/__tests__/mockData/mockClaimData.tsfrontend/__tests__/unit/components/DropdownActions.test.tsxfrontend/__tests__/unit/components/EvidenceActions.test.tsxfrontend/__tests__/unit/components/EvidenceForm.test.tsxfrontend/__tests__/unit/pages/CreateClaimPage.test.tsxfrontend/package.jsonfrontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/evidences/create/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/page.tsxfrontend/src/components/ClaimActions.tsxfrontend/src/components/DropdownActions.tsxfrontend/src/components/EvidenceActions.tsxfrontend/src/components/EvidenceForm.tsxfrontend/src/components/forms/shared/formValidationUtils.tsfrontend/src/types/claim.ts
|
Contributor
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Requires human review: Auto-approval blocked by 8 unresolved issues from previous reviews.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Resolves #5004
PR Depends on #5006 and needs to be rebased.
TODO: update description.
Checklist